feat/bug: replace offset histogram with phrase-performance summary#1514
Open
XaiaX wants to merge 1 commit into
Open
feat/bug: replace offset histogram with phrase-performance summary#1514XaiaX wants to merge 1 commit into
XaiaX wants to merge 1 commit into
Conversation
Replaces the non-applicable hit-offset histogram on vocals score cards with a vocals-appropriate phrase-performance summary: a per-phrase bar histogram with tier-colored background regions, a ranked tier tally, and a percussion hits/total row.
polson
approved these changes
Jun 11, 2026
polson
left a comment
Contributor
There was a problem hiding this comment.
Tested, code and functionality looks good to me.
Just one change for you to make
| { | ||
| // The hit-offset histogram is meaningless for vocals (graded per phrase, not per note); | ||
| // we render a phrase summary in its place instead. | ||
| protected override bool ShouldShowOffsetHistogram => false; |
Contributor
There was a problem hiding this comment.
I think a template method would be a better approach for this:
// Base
protected virtual void BuildAdvancedContent()
{
BuildOffsetHistogram();
}
// VocalsScoreCard
protected override void BuildAdvancedContent()
{
VocalsPhraseHistogram.Build(...);
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The score screen's Advanced view shows a per-note hit-offset histogram (ms early/late) for every instrument — including vocals, where it's not applicable: vocals are graded per phrase on a qualitative scale, not per discrete note with a timing offset. This PR removes the offset histogram for vocals and replaces it with a vocals-appropriate phrase-performance summary:
height is the phrase's normalized score; Awesome phrases render in a gold gradient, everything
else in alternating grays. Tinted background regions and subtle boundary lines mark the grade
tiers (Awful/Messy/Okay/Good/Strong/Awesome — the same tiers as the in-game HUD popup).
always show (even at zero) so multiple vocalists' cards line up row-for-row.
hits / totalfor vocal percussion, shown only when the chart has any;the numerator goes gold on 100%.
No Core changes / replay-version bump
The engine already raises
OnPhraseHitwith the normalized phrase percent, andVocalsPlayersubscribes to it. The per-phrase scores are captured Unity-side as they fire and routed to the score screen throughPlayerScoreCard(GameManager→ScoreScreenMenu→VocalsScoreCard). Nothing is stored in YARG.Core or serialized into replays.The per-phrase list is not persisted in the replay file. The score screen is only reached via a live engine run — either playing normally or via "play with replay." In both cases the engine fires
OnPhraseHitas it plays, populating the summary with no migration. The replay viewer (watch-only) does not reach the score screen.The captures are plain Unity-side lists, not part of the engine state that
BaseStats.Reset()clears.SetReplayTimeandResetPracticeSectionboth re-fireOnPhraseHitandOnNoteHitfrom scratch, so both callResetScoreScreenCaptures()before the engine reprocesses, preventing the re-fired events from appending duplicates. (BaseStats.OffsetSamplesdoesn't need this becauseBaseStats.Reset()already empties it.)Implementation notes
Gameplay/Vocals/VocalPhraseGrade.csholds the tier enum and lower bounds (0.0 / 0.1 / 0.6 / 0.7 / 0.8 / 1.0). These are the same values that were previously hardcoded inVocalsPlayerHUD.GetVocalPerformanceText's inlineswitch; that method now delegates toVocalPhraseGrade.Classify, and the histogram's tier regions/lines readLowerBound(), so the HUD popup and the score screen cannot drift apart. The HUD is a separate consumer — the thresholds live inVocalPhraseGrade, not in the HUD, so changing them means editing one file. The change to the HUD is not purely cosmetic: the oldswitchcompared adoubleagainstfloatliterals (>= 0.6fis really>= 0.60000002…), so a phrase at exactly 0.6 graded Messy;Classifyuses doubles throughout, so it now grades Okay. Affects only values landing exactly on a threshold; arguably a fix.ScoreCardgainsprotected virtual bool ShouldShowOffsetHistogram => true;—VocalsScoreCardoverrides it tofalse, hiding the offset histogram. The base card also exposesAdvancedStatsRect,AdvancedAccentColor, and a card-styledCreateStatLabel(...)factory for subclasses to build their own advanced content. Nothing global is mutated.VocalsPhraseHistogram.Build(...)constructs UGUIImage/RawImagebars, TMP labels, and layout groups at runtime — no prefab changes needed. It builds inside the card'sAdvancedStatsRect, usesAdvancedAccentColorfor the tally divider, and creates labels via the card'sCreateStatLabel(...)factory so they inherit the prefab's text styling. It mirrors the offset histogram's footprint (132px graph height, 54px horizontal margins) and resolves fonts by name from already-loaded assets.VocalsStatsdoesn't track percussion, so hits are counted from engineOnNoteHitfornote.IsPercussion. The total (denominator) is computed once from the chart data at init (NoteTrack.Notes.Sum(phrase => phrase.ChildNotes.Count(n => n.IsPercussion))) rather than accumulated from engine callbacks. This was originally done to work around the shared-mutable-state bug, but is independently correct and will work after a fix. (The bug:CloneAsInstrumentDifficultycopiesVocalNotereferences, so multiple solo-vocals engines on the sameVocalsPartshare the sameVocalNoteobjects. The first engine to hit a percussion note mutatesWasHiton the shared object, causing sibling engines to skip that note. With callback-counted totals, each player saw a different denominator. Chart-derived total is authoritative and identical for all players. The shared-state issue (only one player can actually hit a given percussion note) remains and is in a discord thread, already.Menu.ScoreScreen.PhraseSummaryHeader,….Percussion) inen-US.json; tier labels reuse the existingGameplay.Vocals.Performance.<Tier>keys.Files touched
Gameplay/Player/VocalsPlayer.csGameplay/GameManager.csPlayerScoreCard.Gameplay/HUD/VocalsPlayerHUD.csVocalPhraseGrade.Classify.Gameplay/Vocals/VocalPhraseGrade.cs(new)Classify/LowerBound.Menu/ScoreScreen/ScoreScreenContainer.csPlayerScoreCardfields.Menu/ScoreScreen/ScoreScreenMenu.csVocalsScoreCard.Menu/ScoreScreen/ScoreCards/ScoreCard.csShouldShowOffsetHistogramhook + advanced accessors.Menu/ScoreScreen/ScoreCards/VocalsScoreCard.csMenu/ScoreScreen/ScoreCards/VocalsPhraseHistogram.cs(new)StreamingAssets/lang/en-US.jsonBehavior changes beyond the score screen
Caveats
Texture2Ds are cached in static fields and never destroyed (five 1×256 tier region gradient fills, two 1×256 bar gradients, ~1 KB each). They're created once and reused across score screens for the app lifetime — a deliberate, bounded ~7 KB tradeoff, not a growing leak. TheGetOrCreate*null checks use Unity's overloaded== null, so destroyed textures (e.g. editor play-mode cycles with domain reload off) are recreated correctly.RedHatDisplay-ExtraBold,Barlow-Medium) viaResources.FindObjectsOfTypeAll<TMP_FontAsset>(), with fallback to the label factory's font if not found. Avoids serialized prefab references at the cost of a string match. Cached references are null-checked (not flag-guarded) so destroyed fonts (e.g. domain reload with Enter Play Mode Options) are re-resolved automatically — same pattern as the texture caches above.Testing done